Skip to content

Add reusable OAuth lifecycle custody - #2602

Draft
3mdistal wants to merge 2 commits into
mainfrom
codex/oauth-lifecycle-foundation
Draft

Add reusable OAuth lifecycle custody#2602
3mdistal wants to merge 2 commits into
mainfrom
codex/oauth-lifecycle-foundation

Conversation

@3mdistal

@3mdistal 3mdistal commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Problem

Agent Native integrations each need the same sensitive OAuth lifecycle behavior: credentials must stay bound to the intended provider, resource, and owner; rotating refresh tokens must have one redeemer across concurrent callers; expired or malformed grants must fail into explicit reconnect states; and disconnect must attempt provider revocation without deleting a newer authorization.

Without a shared foundation, later managed AI and Fusion integrations would either duplicate this security logic or inherit the obsolete installation-key design from closed PR #2515.

Approach

Add a provider-agnostic lifecycle layer over the existing encrypted OAuth token store. The public product direction can still present one Builder sign-in, while each future capability keeps a distinct resource-bound grant underneath.

This PR does not add Builder connection UI, managed AI or Fusion consumers, BuilderSync migration, feature enablement, credentials, or deployment changes.

What changed

  • Added resource-derived, owner-scoped credential identity plus explicit missing, malformed, connected, expired, and reconnect_required states.
  • Added revision compare-and-swap writes and atomically increasing row revisions so stale refresh and revocation work cannot overwrite or delete a newer authorization.
  • Added a database-backed refresh lease with heartbeat renewal. One caller redeems a rotating refresh token; concurrent callers wait and reload the winning credential.
  • Made reconnect transitions lease-owned: a stale failed refresher must atomically confirm the lease before marking a credential for reconnect, while a successful result may still win through revision CAS after lease loss.
  • Made owner binding atomic at the upsert itself. Concurrent first saves by different owners cannot silently rebind custody; the loser receives the existing 409 conflict.
  • Added provider revocation hooks that report remote and local outcomes separately and always remove the inspected local credential unless a newer revision replaced it.
  • Adapted existing remote MCP OAuth to the shared lifecycle through an explicit legacy storage-key bridge, preserving exported helper compatibility and validating the requested MCP resource on every token path.
  • Routed MCP OAuth discovery, registration, token exchange, refresh, and every redirect hop through the DNS-aware SSRF guard while preserving method conversion and cross-origin credential stripping. Private origins are allowed only through deployment-owned AGENT_NATIVE_MCP_OAUTH_PRIVATE_ORIGINS configuration.
  • Hardened MCP revocation with the same DNS-aware SSRF guard, HTTPS-only requests, no redirects, and local deletion even when remote revocation fails.

Safety and operations

  • The generic foundation is dormant until a later capability lane calls it; this PR adds no Builder UI or default-on rollout. Existing remote MCP OAuth uses the shared lifecycle behind its unchanged UI.
  • Tokens remain AES-256-GCM encrypted in oauth_tokens. Lease keys contain only a SHA-256 digest of credential identity.
  • Existing MCP rows remain readable through the legacy key bridge. No credential rotation or legacy BuilderSync removal occurs here.
  • Rollback is code-only. Credentials written through the generic API use resource-derived account keys and remain encrypted rows; no Builder consumer writes them in this PR.
  • Provider-controlled OAuth URLs are DNS-checked again at connection time. Request data cannot opt a private origin into the allowlist.
  • Remote revocation never follows redirects or allows discovery-derived private origins. A failed or unsupported provider revocation is reported separately from local cleanup.

Verification

  • Pinned Node 24 focused acceptance run passed 102 tests across OAuth storage/lifecycle, MCP client/routes/remote store, SSRF fetch behavior, and the unchanged MCP connection UI/catalog.
  • pnpm --filter @agent-native/core typecheck passed.
  • pnpm --filter @agent-native/core build passed, including CLI TypeScript and distribution import checks.
  • Credential and product guards passed: guard:no-unscoped-credentials, guard:no-secret-literals, and guard:one-sign-in.
  • A direct SQLite conflict probe verified that a different-owner upsert affects zero rows and preserves the first owner.
  • Independent exact-head auth/security review found three blocking races/boundaries: concurrent owner rebinding, DNS/connect-time SSRF, and stale refresh lease loss. After narrow fixes, final re-review found no remaining blocking issue.
  • The remaining non-blocking evidence gap is unchanged: lease concurrency is exercised in-process against an atomic store mock rather than through truly independent processes and database connections.

Human acceptance boundary

  • There is no UI diff and therefore no changed visual state or honest screenshot to review.
  • The existing MCP OAuth journey remains the only current user-facing adapter: consent and callback, persisted connection and authenticated token use, refresh/concurrency, disconnect/reconnect, cancellation/failure handling, and replay rejection are covered as far as the fixture-backed test surface permits.
  • Independent browser execution is not claimed. The available independent tester could acquire only a PTY, not a trusted browser or separately authorized OAuth test resource.
  • Builder managed-AI consent, workspace membership, reconnect, and capability-specific disconnect UI belong to a later draft consumer lane.

Review focus

  • Does the default resource-derived storage identity provide the right isolation while keeping the MCP legacy bridge narrow enough?
  • Are the lease heartbeat, revision CAS, lease-owned reconnect transition, and waiter reload semantics sufficient for rotating refresh tokens across server processes?
  • Does the DNS-aware MCP OAuth fetch path retain required redirect semantics without permitting request-controlled private access?
  • Does remote revocation balance provider cleanup, SSRF safety, and preservation of a concurrently reauthorized credential?

Follow-ups

  • Add a true multi-process/database-connection concurrency test when a suitable fixture exists.
  • Shape and implement a managed AI capability lane with its own resource/scopes and default-off acceptance story.
  • Shape and implement the Fusion remote MCP lane separately against the landed Builder and AI Services contracts.
  • Keep BuilderSync legacy authentication in place until each replacement lane is proven in production.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Visual recap — skipped

The visual recap job did not run for this pull request. This is informational only and does not block the PR.

Recap skipped for 597d0b5: draft PR.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant